Synchronize what gets compiled into `libc_nonshared.a` with glibc #16970

lifthrasiir · 2023-08-26T10:22:55Z

In lieu of #16152, I tried to compile every glibc version from 2.32 to 2.38 (the most recent) to see any differences. It turns out that recent changes to glibc.zig didn't take account of Makefile changes, and #16152 was only a symptom of this broader issue. This PR updates glibc.zig to take account for the following:

2.33 moved all statically linked stat functions from libc_nonshared.a to libc.so, using the usual ELF symbol versioning. In fact, AFAIK the ELF symbol versioning was a response to the experience from libc_nonshared.a (which can technically violate ODR), and these were the last functions to adapt it. (bminor/glibc@8ed005d, bminor/glibc@589260c, bminor/glibc@4d97cc8, bminor/glibc@99468ed)
- Therefore 19ca241 should be reverted, because 2.32 was the last version that those functions were present in libc_nonshared.a. 3dcd361 can be also safely reverted (they were a simple portable redirection before 2.33).
- In addition, those added files depended on glibc/include/errno.h among others to access __libc_errno. But it is defined from libc.so, so glibc/csu/errno.c should not be added.
-DSHARED option only applies to *.os files, i.e. objects compiled into libc.so. *.oS (note the capital S) files for libc_nonshared.a never needed them and I've verified that glibc 2.32 only passes -DLIBC_NONSHARED=1 for all *.oS files.
- It seems that 6a12dce was a mistake due to the missed 2.33 change, so it should be reverted.
I believe 2.34 is the latest glibc version with non-code changes to libc_nonshared.a. I've checked remaining 4 objects; they all agree with the shipped version except for header inlining.

Now, given this PR deals with multiple glibc versions, I couldn't fully test it. I did however test:

That zig cc -target x86_64-linux-gnu.2.31 still compiles libc_nonshared.a with all stat functions but no errno.o and correctly produces a working shared object for the original example.
That zig cc -target x86_64-linux-gnu.2.34 compiles libc_nonshared.a with only 4 object files remaining since 2.34. I don't have any Linux environment with glibc 2.32 or later so I couldn't directly test it, but at least it correctly compiles with a stat@plt trampoline (as opposed to __xstat@plt) to the stat@GLIBC_2.33 symbol.
That ci/x86_64-linux-release.sh successfully passes. This covers any unintentional changes for non-glibc targets. I intended to produce a full distribution suitable for zig-pypi, so I took some more time.

Any additional verification for other glibc versions and test-std against them would be appreciated.

squeek502 · 2023-10-15T22:40:12Z

This may also fix #17034. Another attempt of a similar fix: #17521

Note: glibc files got updated in #17489

andrewrk · 2023-10-15T23:00:09Z

This is great work! Thanks for doing this. I'm sorry that I did not look at this pull request until now. Are you willing to do the work of rebasing it? If so, let me know. Otherwise, I'll get to it over the next week or so.

lifthrasiir · 2023-10-20T02:09:12Z

I'm sorry that I did not look at this pull request until now.

Oh, it took me a few days to notice this reply as well, don't mind :-)

I'll try to rebase the PR this weekend. If I were not able to do that, you may ignore me and pick this up.

rootbeer

Looks great to me. Nice cleanup in glibc.zig.

The scope of libc_nonshared.a was greatly changed in glibc 2.33 and 2.34, but only the change from 2.34 was reflected so far. Glibc 2.33 finally switched to versioned symbols for stat functions, meaning that libc_nonshared.a no longer contains them since 2.33. Relevant files were therefore reverted to 2.32 versions and renamed accordingly. This commit also removes errno.c, which was probably added to libc_nonshared.a based on a wrong assumption that glibc/include/errno.h requires glibc/csu/errno.c. In reality errno.h should refer to __libc_errno (not to be confused with the public __errno_location), which should be imported from libc.so. The inclusion of errno.c resulted in wrong compile options as well; this commit fixes them as well. Fixes ziglang#16152

Effectively reverts 3dcd361.

rootbeer · 2023-10-23T04:48:46Z

I reproduced the test failures locally on this PR. Its failing to compile the new files when they're used (so the zig build itself works, but building a gnu-libc test case fails):

$ ./build/stage4/bin/zig run ../playground/confused-tls.zig -lc -target native-native-gnu
error(compilation): clang failed with stderr: lib/libc/glibc/io/mknod-2.32.c:55:35: error: expected function body after function declarator

error(compilation): clang failed with stderr: lib/libc/glibc/io/lstat-2.32.c:55:35: error: expected function body after function declarator

error(compilation): clang failed with stderr: lib/libc/glibc/io/stat-2.32.c:54:33: error: expected function body after function declarator

error(compilation): clang failed with stderr: lib/libc/glibc/io/fstat-2.32.c:55:35: error: expected function body after function declarator

error: sub-compilation of glibc libc_nonshared.a failed
    lib/libc/glibc/io/mknod-2.32.c:1:1: note: unable to build C object: clang exited with code 1
    lib/libc/glibc/io/lstat-2.32.c:1:1: note: unable to build C object: clang exited with code 1
    lib/libc/glibc/io/stat-2.32.c:1:1: note: unable to build C object: clang exited with code 1
    lib/libc/glibc/io/fstat-2.32.c:1:1: note: unable to build C object: clang exited with code 1
error: unable to build glibc CRT file: SubCompilationFailed

The line its complaining about is:

weak_hidden_alias (__mknod, mknod)

From what I can tell (manually running the compile command with -E), this line isn't gettting expanded by a macro.... Probably there is some (missing) change between the official v2.32 glibc headers and the ... "custom" headers Zig has ...

Yeah, "weak_hidden_alias" (and a bunch of other macros) were removed from glibc's include/libc-symbols.h in 2022: in https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=include/libc-symbols.h;h=f4437ff6ad770ce79b4beefe08cb9adfa0b6e6da;hp=4fde8394718454d9e11a6485155fa4e7c83922fb;hb=62595e89447c09fe4e34cd9fc1d4cf1b7f3ecb33;hpb=8ee2c043cfb35c48b45c7c5aed4022a8a7352bdc

I'll poke around and see if changing "weak_hidden_alias()" to something else, or putting the header definitions back in makes sense. I may be off base on this, though.

lifthrasiir · 2023-10-23T05:48:49Z

Yeah, "weak_hidden_alias" (and a bunch of other macros) were removed from glibc's include/libc-symbols.h in 2022: in https://sourceware.org/git/?p=glibc.git;a=blobdiff;f=include/libc-symbols.h;h=f4437ff6ad770ce79b4beefe08cb9adfa0b6e6da;hp=4fde8394718454d9e11a6485155fa4e7c83922fb;hb=62595e89447c09fe4e34cd9fc1d4cf1b7f3ecb33;hpb=8ee2c043cfb35c48b45c7c5aed4022a8a7352bdc

Oops, yeah, I wasn't paying attention (my local LLVM compilation is currently out of date, so I only checked histories of conflicting files). I agree that simply reviving weak_hidden_alias and _weak_hidden_alias seems the plausible solution. They essentially define three or four attributes:

__attribute__((weak)) and __attribute__((alias("..."))) should work in any glibc targets (or have common workarounds that don't need additional changes) as it is also a part of the weak_alias macro.
__attribute__((visibility("hidden"))) seems to be similar. It's used by the attribute_hidden macro, which is enabled when SHARED is turned on. So we should be safe unless there is some weird target where only a statically linked glibc is available, in which case we don't need libc_nonshared.a at all.
__attribute__((copy(...))) is indirectly enabled via the __attribute_copy__ macro in cdefs.h for GCC 9+. Assuming __attribute__((alias)) works, this should be safe.

rootbeer · 2023-10-23T06:13:10Z

Pasting the old definitions of weak_hidden_alias back into the libc-symbols.h header seems to fix the problem. Thanks for the pointers.

The patch is available at c76fbfa But its just this diff for libc-symbols.h:

/* Zig patch.  weak_hidden_alias was removed from glibc v2.36 (v2.37?), Zig
   needs it for the v2.32 and earlier {f,l,}stat wrappers, so only include
   in this header for 2.32 and earlier. */
#if (__GLIBC__ == 2 && __GLIBC_MINOR__ <= 32) || __GLIBC__ < 2
# define weak_hidden_alias(name, aliasname) \
  _weak_hidden_alias (name, aliasname)
# define _weak_hidden_alias(name, aliasname) \
  extern __typeof (name) aliasname \
    __attribute__ ((weak, alias (#name), __visibility__ ("hidden"))) \
    __attribute_copy__ (name);
#endif

I wasn't sure if I should push something, or wait for one of you to cherry-pick into this tree. Just redoing the patch is fine with me too.

rootbeer · 2023-10-23T20:42:52Z

I also noticed a test is disabled because of the bug this PR is fixing, so that is probably worth enabling in this stack. There is at least: jacobly0@01f9cdd (a quick search found no others).

andrewrk · 2023-10-25T05:46:56Z

Work is continuing at #17702

squeek502 mentioned this pull request Oct 15, 2023

glibc.zig: update libc_nonshared to match upstream #17521

Closed

andrewrk added this to the 0.12.0 milestone Oct 16, 2023

lifthrasiir force-pushed the correct-libc-nonshared branch from 2ce2233 to 3855b71 Compare October 22, 2023 13:10

rootbeer reviewed Oct 22, 2023

View reviewed changes

lifthrasiir added 2 commits October 22, 2023 15:21

glibc: remove unused stat-related files for 2.33+

982bbc2

Effectively reverts 3dcd361.

andrewrk force-pushed the correct-libc-nonshared branch from 3855b71 to 982bbc2 Compare October 22, 2023 19:21

rootbeer mentioned this pull request Oct 24, 2023

Fix libc_nonshared.a #17702

Merged

andrewrk closed this Oct 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Synchronize what gets compiled into `libc_nonshared.a` with glibc #16970

Synchronize what gets compiled into `libc_nonshared.a` with glibc #16970

lifthrasiir commented Aug 26, 2023

squeek502 commented Oct 15, 2023 •

edited

Loading

andrewrk commented Oct 15, 2023

lifthrasiir commented Oct 20, 2023

rootbeer left a comment

rootbeer commented Oct 23, 2023

lifthrasiir commented Oct 23, 2023

rootbeer commented Oct 23, 2023

rootbeer commented Oct 23, 2023

andrewrk commented Oct 25, 2023

Synchronize what gets compiled into libc_nonshared.a with glibc #16970

Synchronize what gets compiled into libc_nonshared.a with glibc #16970

Conversation

lifthrasiir commented Aug 26, 2023

squeek502 commented Oct 15, 2023 • edited Loading

andrewrk commented Oct 15, 2023

lifthrasiir commented Oct 20, 2023

rootbeer left a comment

Choose a reason for hiding this comment

rootbeer commented Oct 23, 2023

lifthrasiir commented Oct 23, 2023

rootbeer commented Oct 23, 2023

rootbeer commented Oct 23, 2023

andrewrk commented Oct 25, 2023

Synchronize what gets compiled into `libc_nonshared.a` with glibc #16970

Synchronize what gets compiled into `libc_nonshared.a` with glibc #16970

squeek502 commented Oct 15, 2023 •

edited

Loading